-
Notifications
You must be signed in to change notification settings - Fork 12
proper termination, take 2 #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
0dfe9a3
to
e92ca73
Compare
78c2db7
to
475dc90
Compare
475dc90
to
cef292f
Compare
cef292f
to
aa4e09d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, and far simpler than what I was trying. I only noted a couple of places that might benefit from further clarifications. (Overall your docs are excellent, many thanks!)
If you know this works with both Revise and JET, let's get this merged.
e2fcb09
to
b11e388
Compare
This PR is an alternative to #99. This is built on top of #116. With this PR, the following test cases now pass correctly: ```julia #Final block is not a `return`: Need to use #`config::SelectiveEvalRecurse` explicitly ex = quote x = 1 yy = 7 @Label loop x += 1 x < 5 || return yy @goto loop end frame = Frame(ModSelective, ex) src = frame.framecode.src edges = CodeEdges(ModSelective, src) config = SelectiveEvalRecurse() isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges, config) selective_eval_fromstart!(config, frame, isrequired, true) @test ModSelective.x == 5 @test !isdefined(ModSelective, :yy) ``` The basic approach is overloading `JuliaInterpreter.step_expr!` and `LoweredCodeUtils.next_or_nothing!` for the new `SelectiveEvalController` type, as described below, to perform correct selective execution. When `SelectiveEvalController` is passed as the `recurse` argument of `selective_eval!`, the selective execution is adjusted as follows: - **Implicit return**: In Julia's IR representation (`CodeInfo`), the final block does not necessarily return and may `goto` another block. And if the `return` statement is not included in the slice in such cases, it is necessary to terminate `selective_eval!` when execution reaches such implicit return statements. `controller.implicit_returns` records the PCs of such return statements, and `selective_eval!` will return when reaching those statements. This is the core part of the fix for the test cases in #99. - **CFG short-cut**: When the successors of a conditional branch are inactive, and it is safe to move the program counter from the conditional branch to the nearest common post-dominator of those successors, this short-cut is taken. This short-cut is not merely an optimization but is actually essential for the correctness of the selective execution. This is because, in `CodeInfo`, even if we simply fall-through dead blocks (i.e., increment the program counter without executing the statements of those blocks), it does not necessarily lead to the nearest common post-dominator block. And now [`lines_required`](@ref) or [`lines_required!`](@ref) will update the `SelectiveEvalController` passed as their argument to be appropriate for the program slice generated. One thing to note is that currently, the `controller` is not be recursed. That said, in Revise, which is the main consumer of LCU, there is no need for recursive selective execution, and so `selective_eval!` does not provide a system for inter-procedural selective evaluation. Accordingly `SelectiveEvalController` does not recurse too, but this can be left as a future extension.
b11e388
to
04b843a
Compare
end | ||
|
||
An `JuliaInterpreter.Interpreter` that executes only the statements marked `true` in `isrequired`. | ||
Note that this inforeter does not recurse into callee frames. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this inforeter does not recurse into callee frames. | |
Note that this interpreter does not recurse into callee frames. |
Documenter test should be fixed before merging, though |
This PR is an alternative to #99. This is built on top of #116.
With this PR, the following test cases now pass correctly:
The basic approach is overloading
JuliaInterpreter.step_expr!
andLoweredCodeUtils.next_or_nothing!
for the newSelectiveEvalController
type, as described below, to perform correct selective execution.When
SelectiveEvalController
is passed as therecurse
argument ofselective_eval!
, the selective execution is adjusted as follows:Implicit return: In Julia's IR representation (
CodeInfo
), the final block does not necessarily return and maygoto
another block. And if thereturn
statement is not included in the slice in such cases, it is necessary to terminateselective_eval!
when execution reaches such implicit return statements.controller.implicit_returns
records the PCs of such return statements, andselective_eval!
will return when reaching those statements. This is the core part of the fix for the test cases in Add failing test for proper termination #99.CFG short-cut: When the successors of a conditional branch are inactive, and it is safe to move the program counter from the conditional branch to the nearest common post-dominator of those successors, this short-cut is taken. This short-cut is not merely an optimization but is actually essential for the correctness of the selective execution. This is because, in
CodeInfo
, even if we simply fall-through dead blocks (i.e., increment the program counter without executing the statements of those blocks), it does not necessarily lead to the nearest common post-dominator block.And now
lines_required
orlines_required!
will update theSelectiveEvalController
passed as their argument to be appropriate for the program slice generated.One thing to note is that currently, the
controller
is not be recursed. That said, in Revise, which is the main consumer of LCU, there is no need for recursive selective execution, and soselective_eval!
does not provide a system for inter-procedural selective evaluation. AccordinglySelectiveEvalController
does not recurse too, but this can be left as a future extension.